Added connect.timeout() middleware #519
Conversation
…same as a real browser, since safari/chrome/ff all timeout on certain static files.
…le event listener is also only created once.
|
cool thanks! I'll tweak a few little things and merge it over |
| at = req.method + ' ' + req.url, | ||
| timer = setTimeout(function() { | ||
| if (options.throwError) { | ||
| return next(new Error('Timeout ' + req.method + ' ' + req.url)); |
There was a problem hiding this comment.
this actually doesn't work as expected, I'll see if I can find a work-around
There was a problem hiding this comment.
I was curious about how that was expected to work; I just duplicated the old connect-timeout stuff.
There was a problem hiding this comment.
this sort of use-case will work better with #522
I need to think on that some more though, both have pros/cons, deferred errors like this one might invoke app.error several times which is good for reporting but bad for responding. we could tell people to check if (res.headerSent) but that's kinda lame, almost need one callback for reporting errors and one for responding :s but that's sorta lame too
There was a problem hiding this comment.
The only way a timeout should occur is if res.end() is not called within a preset amount of time. This means that there can only be one timeout error. This deferred error can only happen once at most. IMHO, this behavior is desired.
Say an error occurs (i.e. next(err) is called)... the error handler can decide whether or not to call next() or do res.send(500); or whatever. Maybe the error handler prints the message to stderr, or maybe an email is sent. If the error handler decides to call res.send(), then technically, no timeout has happened. So, there is only one error reported to the client. Similarly, only one timeout error can happen.
|
It would also be cool to change the timeout on a per-request basis. Using req.setTimeout(5000) or whatever to clear the current timer and set the new timeout to 5 seconds. This might be cool in case you are doing a large operation, but you have access to the progress of the operation. Or, maybe, different operations might require more time. I'd also suggest proxying the |
Timeout tests included. All tests pass.
re: #499